[#2434] Avoid per-command MDC add+remove in TransportConnection.service - #2435
Conversation
cshannon
left a comment
cshannon
left a comment
There was a problem hiding this comment.
Nice find.
I was thinking this could be simplified and improved even more because we only need to set the value once. We could set it inside the TransportConection start() method after the the transport connection starts, maybe after this line.
The connector is passed in the constructor and is final, so this should never change. If we just set this after start I think you could get rid of the caching of the connectionUri entirely and skip the conditional check, just set it once and be done.
41d6db0 to
3423429
Compare
|
@cshannon I applied the change, but went ahead and moved it up ahead of the brokerInfo being sendAsync(), since there is a chance a service(Command) could come in before it is fully set (ie vm:// transport). |
cshannon
left a comment
cshannon
left a comment
There was a problem hiding this comment.
LGTM, could use a description in the comments on the PR instead of just the code itself to help explain why this change was done
github-project-automation
Bot
moved this from Backlog
to Done
in Apache ActiveMQ v6.4.0
2 participants
MDC entries are copy-on-write maps in log4j2 — a put per command is a
full thread-context map copy per message. Put only when this thread's
tag actually changes and leave it sticky between commands (same
lifecycle-scoped pattern as the activemq.broker MDC entry), so the
steady-state per-command cost is a single allocation-free MDC.get.
In benchmarks this yields 3-6%